Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factories refactor #1843

Merged
merged 28 commits into from
Feb 26, 2024
Merged

Factories refactor #1843

merged 28 commits into from
Feb 26, 2024

Conversation

ludoo
Copy link
Collaborator

@ludoo ludoo commented Nov 6, 2023

This PR is the first step in the change of our approach to factories detailed in the ADR which is also part of this change.

In summary, this PR

  • removes factories from blueprints
  • keeps the blueprints factories README to centralize a high level description of the approach, and list all available module-level factories
  • aligns all factories on the same variable
  • adds pathexpand to all factories
  • moves the project factory to modules and deprecates all other blueprint-level factories
  • removes the centralized firewall blueprint which depended on the firewall (non-module) blueprint factory

@sruffilli
Copy link
Collaborator

+1 to the proposal.

Additionally, I'd like to propose a (permissive) convention for directory/files organization.

  • Factories should consume directories (vs files)
  • All yaml files should contain a dictionary
  • Files in a directory should be parsed together and flattened into a single dictionary

This proposal goes against e.g. our current subnet factory where YaML path and filename are used to extract data, and would give developers freedom to implement multiple resources in a single file, or to use one file per resource, as they see fit.

@ludoo
Copy link
Collaborator Author

ludoo commented Nov 6, 2023

  • Factories should consume directories (vs files)

Yes, agreed with exceptions (the permissive bit you mentioned?), e.g. the cidr file in firewall rules.

  • All yaml files should contain a dictionary

I'm trying to think where we would need a list and failing, so +1.

  • Files in a directory should be parsed together and flattened into a single dictionary

Yep, again with exceptions like the cidr file.

This proposal goes against e.g. our current subnet factory where YaML path and filename are used to extract data, and would give developers freedom to implement multiple resources in a single file, or to use one file per resource, as they see fit.

It's a pattern I kind of like as it allows logical partitioning, and the same for firewall rules where logical partitioning might be useful. But then partitioning can happen at the file name level, and having a (loose) standard is better than having none, so +1.

PS - firewall rules are split into ingress/egress blocks that mimick the variable, and that I would keep so n rules for 1 file
PPS - I would clearly state that factories should mimick and implement the variable interface for the module, including optionals and validation (done in code and checks).

Can you add those bits to the doc by editing this branch?

@sruffilli
Copy link
Collaborator

  • Factories should consume directories (vs files)

Yes, agreed with exceptions (the permissive bit you mentioned?), e.g. the cidr file in firewall rules.

Had the CIDR file in mind when I wrote this, but that doesn't compute as a factory to me. I'm actually having second thoughts on whether that should be a YaML at all vs a variable, but let's postpone/move this conversation.

Can you add those bits to the doc by editing this branch?

On it

@sruffilli
Copy link
Collaborator

Should we break this down into issues and start hammering our keyboards?
I'll have some time starting beginning of dec and keen to work on it.

@drebes
Copy link
Member

drebes commented Nov 22, 2023

Adding here a summary of what we discussed offline.

There are 2 angles for the discussion related to factories:

  1. Factory interfaces, that is, are all the factories consistent on how they handle its inputs? This is relevant as if you're used to some of the existing factories it becomes natural to adopt a new factory. (do not break user expectation related to how our modules work).
  2. Scope - What resources should each factory module manage? This is relevant to determine where the factories should live in the (mono) repo, as the current repo has some implicit definition of what lives where. (do not break user expectations related to how our Fabric repo works).

I'll focus only on the second since I don't have much to add to the interface discussion. For scope, we see that there are generally two types of factories:

  • "Single resource" factories, which manage a partifcular low level component/product and its closely related resources (which are typically managed together). These are just helpers to manage a large number of resources which should be managed as "catle", for example, VPC firewall rules.
  • "Composing" factories which tend to combine multiple different products in a way to solve a particular business problem or process following the "field proven" experience. This typically are related to "platform engineering", that is, a consistent process to onboarding a new workload runtime environment. For example, managing GCP projects, which are not just the project resource itself, but all the resources associated with a particular workload container, like its network subnets, essential contats, VPC SC perimeter membership etc, or a GKE namespace with all its governance rules.

Currently we have the first type of factories in the modules/ and blueprints/ folder, and the second type in blueprints/ and under FAST (fast/stages/). The fact that we have both under blueprints/ is expected since blueprints/ is the "nursery" of composing/modifying modules for new ideas which eventually get graduated into either modules/ or FAST.

So the plan would be that for factories that handle single resources, they should live directly under modules/. For factories that combine resources to implement a process, they should eventually live in FAST.

One of the challenges with having composing factories in FAST is that there is currently a (partly incorrect) assumption that FAST parts (stages) are always to be used to build an end-to-end landing zone, and not to be picked and used independently. One of the reasons for that is that not everything we call a FAST stage is actually a stage. While their folders have numbers suggesting a sequential implementation order, some of them are actually templates/blueprints that need to be customized to the customer to then be applied as one of the sequential stage.

Take project factory for example. A project factory is a FAST stage (which needs to be applied after network and security stages since it expects their outputs as its input when used as a workload "platform engineering" factory, but is also a blueprint that can be used on its own outside of a FAST landing zone as long as its expected inputs are provided via the variable surface (or even in other FAST places like the sandbox stage). In fact, the fast project factory stage is actually a sample implementation for "dev" of the blueprints/ hosted project factory model.

To make this distinction more clear (that these blueprints are a core part of FAST but can also be used on its own), we discussed moving these "template stages" under fast/blueprints/ (the implementation) and keep their actual invocations as a FAST end-to-end stage under fast/stages/. And to improve discoverability, also add references (via documentation) to the blueprints/ folder to these composing factories under fast/blueprints/).

@ludoo
Copy link
Collaborator Author

ludoo commented Nov 22, 2023

Thanks for capturing the earlier discussion Roberto. My only nit is on the new folder naming: fast/stage-blueprints or fast/stage-templates (my preference) instead of fast/blueprints. This would avoid the confusion of "why do we have two blueprint folders" and clearly express the fact these are "stage 3 templates".

@drebes
Copy link
Member

drebes commented Nov 22, 2023

Thanks for capturing the earlier discussion Roberto. My only nit is on the new folder naming: fast/stage-blueprints or fast/stage-templates (my preference) instead of fast/blueprints. This would avoid the confusion of "why do we have two blueprint folders" and clearly express the fact these are "stage 3 templates".

Between fast/stage-blueprints and fast/stage-templates I prefer the first to make it more evident that it's something you can use it on its own without the whole FAST framework. For us (and FAST users) is clear what "stage 3" is but someone using it on its own would be "WTF is stage 3?" :D But in the end either are fine as long as we have the clarification/pointers under the blueprints/ folder. (If you arrive to fast/stage-templates via blueprints/README.md the actual fast folder name is a non-issue.)

@ludoo ludoo changed the title Factories refactor doc Factories refactor Feb 26, 2024
@ludoo ludoo removed request for drebes and wiktorn February 26, 2024 09:25
@ludoo ludoo marked this pull request as ready for review February 26, 2024 09:26
@ludoo ludoo enabled auto-merge (squash) February 26, 2024 10:16
@ludoo ludoo merged commit 6941313 into master Feb 26, 2024
13 checks passed
@ludoo ludoo deleted the ludo/factories-pf-refactor branch February 26, 2024 10:16
@ludoo ludoo restored the ludo/factories-pf-refactor branch February 26, 2024 10:17
@ludoo ludoo added the incompatible change Pull request that breaks compatibility with previous version label Feb 27, 2024
@ludoo ludoo deleted the ludo/factories-pf-refactor branch April 25, 2024 07:43
jayBana pushed a commit to jayBana/cloud-foundation-fabric that referenced this pull request Jul 6, 2024
* factories refactor doc

* Adds file schema and filesystem organization

* Update 20231106-factories.md

* move factories out of blueprints and create new factories  README

* align factory in billing-account module

* align factory in dataplex-datascan module

* align factory in billing-account module

* align factory in net-firewall-policy module

* align factory in dns-response-policy module

* align factory in net-vpc-firewall module

* align factory in net-vpc module

* align factory variable names in FAST

* remove decentralized firewall blueprint

* bump terraform version

* bump module versions

* update top-level READMEs

* move project factory to modules

* fix variable names and tests

* tfdoc

* remove changelog link

* add project factory to top-level README

* fix cludrun eventarc diff

* fix README

* fix cludrun eventarc diff

---------

Co-authored-by: Simone Ruffilli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible change Pull request that breaks compatibility with previous version on:blueprints on:documentation on:FAST on:modules on:tools New or changed tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants